refactor: migrate avoid_using_api#304
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the avoid_using_api lint rule to utilize the analyzer-based RuleContext and RuleVisitorRegistry instead of custom_lint APIs. It replaces AvoidUsingApiLinter with AvoidUsingApiVisitor (extending SimpleAstVisitor), introduces several AST node helper extensions in node_utils.dart, refactors path matching in path_utils.dart, and replaces old integration tests with a comprehensive unit test suite. The review feedback suggests making the rootPath parameter in _getActiveEntries nullable to align with shouldSkipFile and passing context.package?.root.path directly instead of coalescing it to an empty string.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
I found a bug related to the use of severity in this rule, and I'm working on a fix. |
… multiple lint codes
…o 249-migrate-avoid_using_api
| }).toList(); | ||
| } | ||
|
|
||
| List<AvoidUsingApiEntryParameters>? _cachedActiveEntries; |
There was a problem hiding this comment.
Should be before constructor
| final element = node.element; | ||
| if (element is LocalFunctionElement || | ||
| element is TopLevelFunctionElement || | ||
| element is PropertyAccessorElement) { |
There was a problem hiding this comment.
| final element = node.element; | |
| if (element is LocalFunctionElement || | |
| element is TopLevelFunctionElement || | |
| element is PropertyAccessorElement) { | |
| if (node.element | |
| case LocalFunctionElement() || | |
| TopLevelFunctionElement() || | |
| PropertyAccessorElement()) { |
| if (source == null) return; | ||
|
|
||
| final className = entry.className; | ||
| final identifier = entry.identifier; | ||
|
|
||
| switch ((className, identifier)) { | ||
| case (null, null): | ||
| // banSource | ||
| if (matchesSource(node.sourceUrl, source)) { | ||
| reporter.atNode(node, _getLintCode(entry)); | ||
| } | ||
| case (final String className, null): | ||
| // banClassFromSource | ||
| if (node.name.lexeme == className && | ||
| matchesSource(node.sourceUrl, source)) { | ||
| reporter.atNode(node, _getLintCode(entry)); | ||
| } | ||
| default: | ||
| break; | ||
| } |
There was a problem hiding this comment.
| if (source == null) return; | |
| final className = entry.className; | |
| final identifier = entry.identifier; | |
| switch ((className, identifier)) { | |
| case (null, null): | |
| // banSource | |
| if (matchesSource(node.sourceUrl, source)) { | |
| reporter.atNode(node, _getLintCode(entry)); | |
| } | |
| case (final String className, null): | |
| // banClassFromSource | |
| if (node.name.lexeme == className && | |
| matchesSource(node.sourceUrl, source)) { | |
| reporter.atNode(node, _getLintCode(entry)); | |
| } | |
| default: | |
| break; | |
| } | |
| if (source == null || | |
| entry.identifier != null || | |
| entry.className == null || | |
| !matchesSource(node.sourceUrl, source)) { | |
| return; | |
| } | |
| reporter.atNode(node, _getLintCode(entry)); |
| final source = entry.source; | ||
| if (source == null) return; | ||
|
|
||
| final className = entry.className; | ||
|
|
||
| switch ((className, entry.identifier)) { | ||
| case (final String className, null): | ||
| // banClassFromSource | ||
| final typeElement = node.declaredType?.element; | ||
| if (typeElement?.name == className && | ||
| matchesSource(typeElement?.libraryUri, source)) { | ||
| reporter.atOffset( | ||
| offset: node.name.offset, | ||
| length: node.name.length, | ||
| diagnosticCode: _getLintCode(entry), | ||
| ); | ||
| } | ||
| default: | ||
| break; | ||
| } |
There was a problem hiding this comment.
| final source = entry.source; | |
| if (source == null) return; | |
| final className = entry.className; | |
| switch ((className, entry.identifier)) { | |
| case (final String className, null): | |
| // banClassFromSource | |
| final typeElement = node.declaredType?.element; | |
| if (typeElement?.name == className && | |
| matchesSource(typeElement?.libraryUri, source)) { | |
| reporter.atOffset( | |
| offset: node.name.offset, | |
| length: node.name.length, | |
| diagnosticCode: _getLintCode(entry), | |
| ); | |
| } | |
| default: | |
| break; | |
| } | |
| final source = entry.source; | |
| final className = entry.className; | |
| final typeElement = node.declaredType?.element; | |
| if (source == null || | |
| entry.identifier != null || | |
| className == null || | |
| typeElement?.name != className || | |
| !matchesSource(typeElement?.libraryUri, source)) { | |
| return; | |
| } | |
| reporter.atOffset( | |
| offset: node.name.offset, | |
| length: node.name.length, | |
| diagnosticCode: _getLintCode(entry), | |
| ); |
| if (actualClassName != className) { | ||
| return; | ||
| } | ||
|
|
||
| if (node.constructorName.name?.name != expectedConstructorName) { | ||
| return; | ||
| } | ||
|
|
||
| if (!node.argumentList.containsNamed(namedParameter)) { | ||
| return; | ||
| } | ||
|
|
||
| if (matchesSource( | ||
| node.constructorName.type.element?.libraryUri, | ||
| source, | ||
| )) { |
There was a problem hiding this comment.
| if (actualClassName != className) { | |
| return; | |
| } | |
| if (node.constructorName.name?.name != expectedConstructorName) { | |
| return; | |
| } | |
| if (!node.argumentList.containsNamed(namedParameter)) { | |
| return; | |
| } | |
| if (matchesSource( | |
| node.constructorName.type.element?.libraryUri, | |
| source, | |
| )) { | |
| if (actualClassName == className && | |
| node.constructorName.name?.name == expectedConstructorName && | |
| node.argumentList.containsNamed(namedParameter) && | |
| matchesSource( | |
| node.constructorName.type.element?.libraryUri, | |
| source, | |
| )) { |
| if (element == null) return false; | ||
|
|
||
| final target = element is InterfaceElement | ||
| ? element | ||
| : element.enclosingElement; | ||
| if (target == null) return false; | ||
|
|
||
| if (target is InterfaceElement || target is ExtensionElement) { | ||
| return target.name == className && | ||
| matchesSource(target.libraryUri, source); | ||
| } | ||
|
|
||
| return false; |
There was a problem hiding this comment.
| if (element == null) return false; | |
| final target = element is InterfaceElement | |
| ? element | |
| : element.enclosingElement; | |
| if (target == null) return false; | |
| if (target is InterfaceElement || target is ExtensionElement) { | |
| return target.name == className && | |
| matchesSource(target.libraryUri, source); | |
| } | |
| return false; | |
| final target = element is InterfaceElement | |
| ? element | |
| : element?.enclosingElement; | |
| if (target case InterfaceElement() || ExtensionElement() | |
| when target != null) { | |
| return target.name == className && | |
| matchesSource(target.libraryUri, source); | |
| } | |
| return false; |
There was a problem hiding this comment.
There are probably more similar refactors possible in this file that I missed
There was a problem hiding this comment.
Thank you! I implemented similar improvements in other parts of this class as well.
…ingApiVisitor to improve readability and structure
Closes #249